Skip to content

Add scons options for compilation verbosity - #104

Open
clemisch wants to merge 4 commits into
diffpy:mainfrom
clemisch:compile_silent
Open

Add scons options for compilation verbosity#104
clemisch wants to merge 4 commits into
diffpy:mainfrom
clemisch:compile_silent

Conversation

@clemisch

Copy link
Copy Markdown
Contributor

(essentially the same as diffpy/libobjcryst#32)

Currently the manual compilation with scons dev is quite noisy: it prints all compiler commands and all compiler warnings. There is no flag to control it.

For development I found that annoying, mainly because it's hard to spot an error in the wall of warnings.

This PR adds scons flags verbose: true|false = false and warnigns: all|default|none = none. verbose controls if compiler commands are printed. warnings controls compiler warnings. For the latter:

  • all: inherited -W flags (from python-config) + -Wextra
  • default: no explicit warning-control flags
  • none: suppress all warnings (only show errors) [new default]

I think it's more common to show warnings on demand rather than by default, but that's kinda subjective and I'm happy to change the default to warnings=all and verbose=true. That would mimick current behavior.

@clemisch

Copy link
Copy Markdown
Contributor Author

I'll add the news item after we discussed the rest and came to an agreement ;-)

@vincefn

vincefn commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

If we move towards nanobind, the proposed branch switched to cmake, so it may supersede this. And it would remove libobjcryst. I guess that needs to be settled first.

@clemisch

Copy link
Copy Markdown
Contributor Author

Yeah but how soon will that be, realistically?

@sbillinge

Copy link
Copy Markdown
Contributor

We are pushing hard on the nanobind migration for pdffit2, and diffpy because we weren't able to release at 3.14 with boost. We could either put effort into that, or the nanobind migration and given our limited bandwidth we prefer the latter. But we could use some additional c++ eyes on the code. So far, we found it wasn't such a huge lift, but we don't have much testing done yet.

@clemisch when it comes time to make the news, consider installing scikit-package and using package add news -f "Fixed such and such". Also you can ask Claude to read the Billinge group coding standards in the documentation of scikit-package and it will make the news for you too.

@vincefn

vincefn commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

@sbillinge this looks good to me. I normally merge changes which are only affecting limited functionality in pyobjcryst, but as this one touches the build system and I do not know how it integrates with your diffpy overall build, I let you do the merge.

@sbillinge

Copy link
Copy Markdown
Contributor

@stevenhua0320 @stevengeorge128 @SYMSCAE @cadenmyers13 if any of you can volunteer to try and do a build from this branch and make sure these changes don't break the build, it would be great. We want to mimic what happens in the auto build on GH actions, but in particular it is just the build steps. I think if it works on one platform and one python version we can merge and try a matrix build using GH actions to do a full test.

@SYMSCAE

SYMSCAE commented Aug 26, 2026

Copy link
Copy Markdown

Hi @clemisch , probably it's better to set default warning level as "default"? Also I would hesitate on suppressing linker warnings. Please see if that makes sense, thanks!

@clemisch

Copy link
Copy Markdown
Contributor Author

@SYMSCAE Sure, I think it makes sense that you folks decide the defaults! I just want the option to control it :-) Feel free to push changes in this branch, should be enabled already.

@sbillinge

Copy link
Copy Markdown
Contributor

Thanks @SYMSCAE ! I think that, greater verbosity is probably better for us so that if a build fails we see everything. We basically never look at the logs unless something fails. So I think it would be good to add back linker warnings and maybe make the default back to default if we can as the default option.

@clemisch

Copy link
Copy Markdown
Contributor Author

I changed defaults to verbose=true and warnings=all. That should lead to same default behavior (print compiler commands and warnings).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants